feat: ignore test_and_release in PRs without code changes by robertolopezlopez · Pull Request #6974 · snyk/cli

robertolopezlopez · 2026-07-08T08:37:13Z

User description

Pull Request Submission Checklist

Follows CONTRIBUTING guidelines
Commit messages
are release-note ready, emphasizing
what was changed, not how.
Includes detailed description of changes
Contains risk assessment (Low | Medium | High)
Highlights breaking API changes (if applicable)
Links to automated tests covering new functionality
Includes manual testing instructions (if necessary)
Updates relevant GitBook documentation (PR link: ___)
Includes product update to be announced in the next stable release notes

What does this PR do?

It skips the test_and_release pipeline for changes which do not touch code nor dependencies. Keeps the previous behaviour for others.

Where should the reviewer start?

How should this be manually tested?

Right now the pipeline is running full in this PR.

After merge to main, there will be another PR showcasing the "light scenario".

What's the product update that needs to be communicated to CLI users?

PR Type

Enhancement

Description

Detect code changes vs. docs-only PRs.
Conditionally run full or lightweight CI.
Document new CI workflow behavior.

Diagram Walkthrough

flowchart LR
  A[CI Trigger] --> B{Detect Code Changes};
  B -- Code Changes Found --> C[Full CI Pipeline];
  B -- Docs-Only Changes --> D[Lightweight CI Pipeline];

File Walkthrough

Relevant files

Enhancement

7 files

agent.go `Changes required by linter, also failing in main branch`	+2/-4
has-code-changes.sh `New script to detect code changes`	+32/-0
is-docs-only-changes.sh `New script to identify documentation-only changes`	+24/-0
list-changed-files.sh `Helper script to list changed files`	+13/-0
config.yml `Implement CI job for detecting code changes`	+37/-2052
continue_config.yml `New file for full CI pipeline configuration`	+2054/-0
continue_config_light.yml `New file for lightweight CI pipeline configuration`	+14/-0

Bug fix

1 files

interactive_test.go `Changes required by linter, also failing in main branch`	+9/-3

Tests

2 files

test_has_code_changes.sh `Tests for code change detection script`	+76/-0
test_is_docs_only_changes.sh `Tests for documentation-only change detection`	+58/-0

Documentation

1 files

CONTRIBUTING.md `Document new CI pipeline behavior and logic`	+32/-0

Configuration changes

1 files

code-change-pathspecs.txt `Define pathspecs for code changes`	+15/-0

snyk-io · 2026-07-08T08:37:40Z

✅ Snyk checks have passed. No issues have been found so far.

Status	Scan Engine	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues
✅	Licenses	0	0	0	0	0 issues
✅	Code Security	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

robertolopezlopez · 2026-07-08T11:33:50Z

/describe

snyk-pr-review-bot · 2026-07-08T11:36:38Z

PR Description updated to latest commit (506ea98)

graphite-app · 2026-07-08T11:50:15Z

+            if ./scripts/ci/has-code-changes.sh; then
+              echo '.circleci/continue_config.yml' > /tmp/continuation-config-path.txt
+            else
+              echo '.circleci/continue_config_light.yml' > /tmp/continuation-config-path.txt
            fi


Error handling bug: If has-code-changes.sh exits with error code 2 (e.g., when git merge-base fails due to insufficient fetch depth), the else branch executes and selects the lightweight config instead of failing the build. This silently masks errors and could run the wrong pipeline.

Impact: On older PR branches where the merge-base is beyond the 100-commit fetch depth (line 17), the detection will error but incorrectly proceed with lightweight config, skipping critical tests.

Fix:

set +e ./scripts/ci/has-code-changes.sh result=$? set -e if [ $result -eq 0 ]; then echo '.circleci/continue_config.yml' > /tmp/continuation-config-path.txt elif [ $result -eq 1 ]; then echo '.circleci/continue_config_light.yml' > /tmp/continuation-config-path.txt else echo "Error detecting code changes (exit $result)" >&2 exit 1 fi

Suggested change

if ./scripts/ci/has-code-changes.sh; then

echo '.circleci/continue_config.yml' > /tmp/continuation-config-path.txt

else

echo '.circleci/continue_config_light.yml' > /tmp/continuation-config-path.txt

fi

set +e

./scripts/ci/has-code-changes.sh

result=$?

set -e

if [ $result -eq 0 ]; then

echo '.circleci/continue_config.yml' > /tmp/continuation-config-path.txt

elif [ $result -eq 1 ]; then

echo '.circleci/continue_config_light.yml' > /tmp/continuation-config-path.txt

else

echo "Error detecting code changes (exit $result)" >&2

exit 1

fi

Spotted by Graphite

Is this helpful? React 👍 or 👎 to let us know.

danskmt · 2026-07-08T12:09:34Z

 func canonicalAgent(name string) Agent {
-	switch Agent(name) {
-	case "github-copilot-cli":
+	if Agent(name) == "github-copilot-cli" {
 		return AgentGitHubCopilot
-	default:
-		return Agent(name)
 	}
+	return Agent(name)
 }


some changes such as this one I believe is not necessary in this PR

yes this is necessary because linter was failing

danskmt · 2026-07-08T12:10:42Z

+	defer func(r *os.File) {
+		_ = r.Close()
+	}(r)
+	defer func(w *os.File) {
+		_ = w.Close()
+	}(w)


Why was this change needed?

danskmt · 2026-07-08T12:58:36Z

+BASE_REF="${1:-${BASE_REF:-origin/main}}"
+
+if ! git rev-parse --verify "$BASE_REF" >/dev/null 2>&1; then
+  echo "list-changed-files: unknown base ref: $BASE_REF" >&2
+  exit 2
+fi
+
+MERGE_BASE="$(git merge-base HEAD "$BASE_REF")"
+git diff --name-only "${MERGE_BASE}...HEAD"


can probably reuse what is being used in has-code-changes.sh

danskmt · 2026-07-08T13:16:15Z

+set -euo pipefail
+
+SCRIPT_DIR="$(CDPATH= cd -- "$(dirname -- "$0")" && pwd)"
+ALLOWED='(\.md|\.svg|\.jpg)$'


Also, instead of maintaining this separate list, can we just make an "opposite" of the patterns we have in the other file? (scripts/ci/code-change-pathspecs.txt)

snyk-pr-review-bot · 2026-07-08T13:56:58Z

PR Reviewer Guide 🔍

🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review Incomplete Asset Allowlist 🟡 [minor] The `ALLOWED` regex for docs-only changes is restricted to `.md`, `.svg`, and `.jpg`. This excludes common documentation image formats like `.png` or `.gif`. The `CONTRIBUTING.md` file specifically mentions `help/snyk-cli-screenshot.png` as a repository asset. If a contributor updates this screenshot, the CI will fail the docs-only check and force the full pipeline, or the script will exit 1 and potentially stall the setup job. ALLOWED='(\.md\|\.svg\|\.jpg)$' Deferred Error Ignoring 🟡 [minor] The updated tests wrap `r.Close()` and `w.Close()` in anonymous functions to ignore the error (using `_ =`). While acceptable in tests, the original code `defer r.Close()` was idiomatic for these specific OS file descriptors in a test context. More importantly, using `defer` on `os.Pipe()` descriptors inside a loop or function that might be called many times can lead to descriptor exhaustion if not careful, though it is safe in these specific unit tests. defer func(r os.File) { _ = r.Close() }(r) defer func(w os.File) { _ = w.Close() }(w)
📚 Repository Context Analyzed This review considered 20 relevant code sections from 13 files (average relevance: 0.57) release-scripts/upload-artifacts.sh (lines 1-199) scripts/format/prettier-changes.sh (lines 1-27) release-scripts/skip-deployment.sh (lines 1-20) CLAUDE.md (lines 1-148) release-scripts/validate-repository.sh (lines 1-63) dangerfile.js (lines 1-111) cliv2/internal/persona/interactive/interactive.go (lines 1-64) ... and 6 more file(s)
🤖 Repository instructions applied (from `AGENTS.md`)

robertolopezlopez force-pushed the feat/CLI-1625 branch 3 times, most recently from 4944ced to 5c91d33 Compare July 8, 2026 09:23

robertolopezlopez changed the title ~~Feat/cli 1625~~ feat: ignore test_and_release in PRs without code changes Jul 8, 2026

robertolopezlopez force-pushed the feat/CLI-1625 branch from 5d2253f to 506ea98 Compare July 8, 2026 11:33

robertolopezlopez force-pushed the feat/CLI-1625 branch from 506ea98 to 75e5ac5 Compare July 8, 2026 11:39

robertolopezlopez marked this pull request as ready for review July 8, 2026 11:46

robertolopezlopez requested a review from a team as a code owner July 8, 2026 11:46

This comment has been minimized.

Sign in to view

graphite-app Bot reviewed Jul 8, 2026

View reviewed changes

danskmt reviewed Jul 8, 2026

View reviewed changes

feat: ignore test_and_release in PRs without code changes

80ad6e1

robertolopezlopez force-pushed the feat/CLI-1625 branch from 75e5ac5 to 80ad6e1 Compare July 8, 2026 13:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: ignore test_and_release in PRs without code changes#6974

feat: ignore test_and_release in PRs without code changes#6974
robertolopezlopez wants to merge 1 commit into
mainfrom
feat/CLI-1625

robertolopezlopez commented Jul 8, 2026 •

edited

Loading

Uh oh!

snyk-io Bot commented Jul 8, 2026 •

edited

Loading

Uh oh!

robertolopezlopez commented Jul 8, 2026

Uh oh!

snyk-pr-review-bot Bot commented Jul 8, 2026

Uh oh!

This comment has been minimized.

graphite-app Bot Jul 8, 2026

Uh oh!

danskmt Jul 8, 2026

Uh oh!

robertolopezlopez Jul 8, 2026

Uh oh!

danskmt Jul 8, 2026

Uh oh!

robertolopezlopez Jul 8, 2026

Uh oh!

danskmt Jul 8, 2026

Uh oh!

danskmt Jul 8, 2026 •

edited

Loading

Uh oh!

snyk-pr-review-bot Bot commented Jul 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

robertolopezlopez commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

User description

Pull Request Submission Checklist

What does this PR do?

Where should the reviewer start?

How should this be manually tested?

What's the product update that needs to be communicated to CLI users?

PR Type

Description

Diagram Walkthrough

File Walkthrough

Uh oh!

snyk-io Bot commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Snyk checks have passed. No issues have been found so far.

Uh oh!

robertolopezlopez commented Jul 8, 2026

Uh oh!

snyk-pr-review-bot Bot commented Jul 8, 2026

Uh oh!

This comment has been minimized.

graphite-app Bot Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

danskmt Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

robertolopezlopez Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

danskmt Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

robertolopezlopez Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

danskmt Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

danskmt Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

snyk-pr-review-bot Bot commented Jul 8, 2026

PR Reviewer Guide 🔍

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

robertolopezlopez commented Jul 8, 2026 •

edited

Loading

snyk-io Bot commented Jul 8, 2026 •

edited

Loading

danskmt Jul 8, 2026 •

edited

Loading